Skip to content

ADFA-5048 (3/5): Split the region analysis primitives into one file per concern - #1819

Open
Daniel-ADFA wants to merge 1 commit into
feat/ADFA-5048-extraction-regionfrom
feat/ADFA-5048-region-analysis-primitives
Open

ADFA-5048 (3/5): Split the region analysis primitives into one file per concern#1819
Daniel-ADFA wants to merge 1 commit into
feat/ADFA-5048-extraction-regionfrom
feat/ADFA-5048-region-analysis-primitives

Conversation

@Daniel-ADFA

@Daniel-ADFA Daniel-ADFA commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Stack 3 of 5 for ADFA-5048. Base: #1818. Mechanical split, no logic change; no caller until PR 4.

Splits the analysis primitives out of what would otherwise be a single 1000-line file, one file per concern, where a concern is a set of functions that only meet at the entry point:

  • AnchorMember.kt - where the new declaration goes, and whether it must be static.
  • RegionReferences.kt - what the region names, which of those are captures, and what it reassigns.
  • RegionTypes.kt - rendering a type as source, and the shapes that cannot be written out.

The functions are moved verbatim by a block parser rather than retyped, so no comment is orphaned and no import dropped. private was promoted to internal only for the symbols now referenced across a file boundary; isStaticMember and MAX_TYPE_DEPTH stayed private.

This branch compiles on its own (:lsp:java:compileV8DebugKotlin), which is the point of cutting here.

The three concerns extract method's analysis needs before it can derive a
signature, each independent of the others and of the plan model:

- AnchorMember.kt - the class member the new method becomes a sibling of.
  "Nearest ancestor that is a direct member of a ClassTree" covers method,
  constructor, initializer and field uniformly, and decides `static`.
- RegionReferences.kt - what the region names: every identifier in source
  order, which of those are locals declared inside the anchor but outside the
  region (the future parameters), and whether the region reassigns one it does
  not declare.
- RegionTypes.kt - rendering a type as source, shortened only where the file
  already resolves the short form, plus the two shapes that cannot be written
  out: a local or anonymous class, and a type variable owned by the anchor.

Split into three files rather than one because they only meet in the entry
point that follows, and 1000 lines in one file is not reviewable.

No caller yet; the analysis that uses them lands next.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 2e828679-f180-49f1-b9ff-1d20f18df1f6

📥 Commits

Reviewing files that changed from the base of the PR and between b095c4f and 86b5e1e.

📒 Files selected for processing (3)
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/refactor/AnchorMember.kt
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/refactor/RegionReferences.kt
  • lsp/java/src/main/java/com/itsaky/androidide/lsp/java/refactor/RegionTypes.kt

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Summary
  • Split region analysis primitives into AnchorMember.kt, RegionReferences.kt, and RegionTypes.kt.
  • Preserved existing logic and callers.
  • Changed selected declarations from private to internal for cross-file access.
  • Kept isStaticMember and MAX_TYPE_DEPTH private.
  • Verified compilation with :lsp:java:compileV8DebugKotlin.
  • Risk: The change increases the internal visibility of selected declarations.
  • Risk: The summary reports compilation validation only. It does not report dedicated behavioral tests.

Walkthrough

Added Java refactoring utilities to resolve enclosing members, analyze captured references and mutations, detect relevant type constructs, and render resolvable type names for extraction regions.

Changes

Java refactoring analysis

Layer / File(s) Summary
Anchor member resolution
lsp/java/src/main/java/com/itsaky/androidide/lsp/java/refactor/AnchorMember.kt
Resolves the nearest direct class member for an extraction region. It records member paths, spans, static status, and optional method declarations.
Region reference analysis
lsp/java/src/main/java/com/itsaky/androidide/lsp/java/refactor/RegionReferences.kt
Collects ordered identifier references, detects captured local variables and direct reassignment, and resolves declaration spans with safe failure handling.
Region type analysis
lsp/java/src/main/java/com/itsaky/androidide/lsp/java/refactor/RegionTypes.kt
Detects method type variables and referenced local or anonymous classes across nested type shapes. TypeNames renders resolvable names using file imports.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 86b5e

This change reorganizes existing region-analysis code into separate files without changing behavior or callers, and the affected module compiles successfully. It is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the ADFA-5048 stack step and the main change: splitting region analysis primitives into separate files.
Description check ✅ Passed The description directly explains the mechanical file split, the responsibilities of each file, the visibility changes, and the compilation check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ADFA-5048-region-analysis-primitives

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants